Size Change: +223 B (0%)

Total Size: 1.24 MB

_{compressed-size-action}

I tested this and it adds the ESC behaviour. I also noticed that:

maybe related to this PR

• pressing TAB when a submenu is open will focus the next tabbable element, then move the focus back to the submenu parent. I think this is because of our focus management here. May not be an issue but I think the quick focus swap may be announced?

unrelated to this PR

• pressing TAB on a page to get to the menu, only selects the submenu, and the menu cannot be - at least I can't figure out how - navigated with the keyboard: TAB will tab away from the menu, the arrow keys scroll.

@draganescu My thoughts.

pressing TAB when a submenu is open will focus the next tabbable element, then move the focus back to the submenu parent. I think this is because of our focus management here. May not be an issue but I think the quick focus swap may be announced?

I don't think this is really an issue, actually, one could argue it should have been working like this all along. The code that handles this detects outside focus, the only thing I changed was adding the Escape key. As a result, the focus was necessary to prevent Escape key from closing the menu and focus getting trapped. If others think it is a big UX issue, I could always split out the functions and only focus on Escape key vs. loss of focus on the menu.

We discussed this Pull Request during the weekly Accessibility Team's bug-scrub: discussion happened in the accessibility channel on Slack.
The Team sees this change favourably: having a quick way to leave a submenu is extremely useful, especially when there are more that a few items in it.
The fact that focus is moved back to the submenu trigger is not a big issue either: it's true that it's an extra tab stop, but it's actually announced to users because of the change in the aria-expanded attribute.

@draganescu I am sure we could but then it would not announce the closed state. If this was the case, users should reasonably expect to be able to Shift+Tab back to the open submenu only finding that it closed with no notification to them. I am not sure those UX would be any better.

Soon to become the most approved PR on Github 😂

Thanks for the reviews everyone! If there is any further questions about UX, happy to make adjustments should they be needed.

Late to the party, but I noticed a couple issues with this change.
The first part, close on Escape key press, makes perfectly sense.
The second part, always move focus to the trigger, may trigger some unexpected behaviors that are pretty confusing. I'll create a new issue with more details.